Reader tab: style recent trades like Activity tab FeedRow#795
Merged
realproject7 merged 2 commits intomainfrom Apr 3, 2026
Merged
Reader tab: style recent trades like Activity tab FeedRow#795realproject7 merged 2 commits intomainfrom
realproject7 merged 2 commits intomainfrom
Conversation
Restyle HoldingRecentTrades with bordered card rows, color-coded Buy/Sell labels (green/red), USD amounts, date, and Basescan tx link — matching the Activity tab FeedRow pattern. Fixes #786 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
requested changes
Apr 3, 2026
Collaborator
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The visual restyle is aligned with the Activity tab pattern, and the surrounding holding card already provides the story link. One acceptance criterion from issue #786 is still missing.
Findings
- [medium] The timestamp is still rendered as plain text instead of a clickable link, so the PR does not satisfy issue #786 item 4 ("Date: Linked as a clickable timestamp").
- File:
src/app/profile/[address]/page.tsx:1607 - Suggestion: Wrap the date in an anchor, using the intended permalink target for the trade instead of a bare
<time>element.
- File:
Decision
Requesting changes until the date is actually clickable, which is still part of the issue's stated acceptance criteria.
Date + arrow now wrapped in a single anchor linking to the tx on Basescan. Falls back to plain <time> when tx_hash is missing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
approved these changes
Apr 3, 2026
Collaborator
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up change resolves the remaining acceptance-criteria gap from issue #786. The recent-trades row now matches the intended Activity-tab pattern closely enough to merge.
Findings
- [resolved] The date is now a clickable timestamp when
tx_hashis present, with a safe plain<time>fallback when there is no transaction link target.- File:
src/app/profile/[address]/page.tsx:1607 - Suggestion: None.
- File:
Decision
Approving. My prior blocker on the non-clickable timestamp is addressed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HoldingRecentTradesto match Activity tabFeedRowpatternborder-border rounded border px-3 py-1.5text-green-700) / red (text-red-700)formatUsdValue<time>elementtx_hashfromtrade_historyquery (was not queried before)Test Plan
Fixes #786
🤖 Generated with Claude Code